Skip to content

Use SpatialConstant naming in get_pde_as_diff_op - #309

Open
alexfikl wants to merge 3 commits into
inducer:mainfrom
alexfikl:fix-get-pde-as-diff-op
Open

alexfikl wants to merge 3 commits into
inducer:mainfrom
alexfikl:fix-get-pde-as-diff-op

Conversation

@alexfikl

@alexfikl alexfikl commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

This came up in #279 because the Fourier symbol was derived from the expression returned by get_pde_as_diff_op, but the kernel itself (e.g. the global scaling) used the SpatialConstant naming and conflicted (i.e. could not be simplified).

This seems like a good idea to keep it consistent?

@alexfikl
alexfikl force-pushed the fix-get-pde-as-diff-op branch from 1a83395 to 2e5ee87 Compare October 2, 2026 16:33
@alexfikl
alexfikl marked this pull request as draft October 2, 2026 16:33
@alexfikl
alexfikl marked this pull request as ready for review October 2, 2026 16:35
@alexfikl
alexfikl force-pushed the fix-get-pde-as-diff-op branch from 2e5ee87 to 024aaf6 Compare October 2, 2026 16:50
@alexfikl

alexfikl commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

@inducer From a quick grep, it seems like get_bessel_arg_scaling in here is the only place that doesn't go through SpatialConstant for the symbol name. Should I change that as well?

def get_bessel_arg_scaling(self) -> sym.Expr:
from sumpy.kernel import HelmholtzKernel
kernel = self.kernel.get_base_kernel()
assert isinstance(kernel, HelmholtzKernel)
return sym.Symbol(kernel.helmholtz_k_name)

@alexfikl
alexfikl force-pushed the fix-get-pde-as-diff-op branch from 024aaf6 to 6951205 Compare October 9, 2026 14:13
@inducer

inducer commented Oct 9, 2026

Copy link
Copy Markdown
Owner

Yes, let's that make that a SpatialConstant as well.

@alexfikl
alexfikl force-pushed the fix-get-pde-as-diff-op branch from 6951205 to f666c56 Compare October 9, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants